Let long subject tags wrap instead of shoving the book page sideways (#1170)#1171
Merged
Merged
Conversation
Rolls the eight user-facing entries sitting in [Unreleased] into a [v4.1.22] section and adds the matching in-app feature log block, so the shipped image carries the release notes that announce it. Two gaps found while reconciling the changelog against git log v4.1.21..HEAD: - The book-card series-line contrast fix (7945918, issue #1135) was user-facing but had no changelog entry and no CHANGES row. Both added. - The read-indicator change (0e423fb, issue #1117) had a changelog entry but no CHANGES row. Added. Release cells: the five rows shipping now are stamped v4.1.22. Two older rows (#1090, #1091) still read TBD despite being reachable from v4.1.21 — git tag --contains puts both in that release, so they are backfilled v4.1.21 rather than swept into this one. What's New folds #1150 and #1121 into a single item; one commit (d50be75) fixes both from one root cause and they are the same symptom to a reader. Seven items, English-authored, deep links verified against SPA_ROUTES; "Browse languages" anchored in spa_strings.py so the CTA survives msgid re-extraction.
The read-only tag pill was `white-space: nowrap` with no `max-width`, so its intrinsic width could exceed the wrapping flex row that holds it. A library carrying Library-of-Congress subject headings — `France -- History -- Revolution, 1789-1799 -- Fiction` and friends, which is most metadata imported from public-domain sources — pushed the whole book detail page past the viewport, and the page scrolled left/right on a phone. Only the no-edit-role render is affected. Accounts with edit get the removable chip row, which wraps already. Every mobile spec logs in as admin, so the gate never covered the render that guests and viewer accounts actually get. Measured on a stock library as a guest: 12px of overflow at 390px, 82px at 320px, 323px with one unbroken token. Keeping `nowrap` and adding `max-width` alone leaves 82px, which is what pins `nowrap` as the cause rather than a missing cap. Pills that fit still sit on one line — normal wrapping only breaks a line that does not fit — so short pills are unchanged at 29px tall and only over-long ones grow to two lines. `overflow-wrap: anywhere` covers a single token wider than the row. The regression test stubs the role and the tag list, so it holds on any seed rather than depending on the fixture happening to carry a long heading.
new-usemame
added
the
safe-tier-2
github-actions
Bot
added
needs-review
Contributor
|
🤖 auto-merge skipped: PR is too large for safe-tier-2 (additions=114, files=6; caps are 80 LOC / 3 files). Demoted to needs-review. |
The first version of this spec asserted zero horizontal overflow outright. In CI it failed with 35px — the same figure `hidden-books.spec.ts:252` reports, on a different spec and a different navigation, with the tag row stubbed to a known state. The CI fixture's detail page carries ~35px of overflow from a cause that has nothing to do with tags (notes/e2e-mobile-overflow-ci-triage.md). An absolute assertion there is permanently red for an unrelated reason and says nothing about the pills. Measure the delta instead: render the page with its own tags, measure, swap in the pathological ones, measure again, and require the long tags to add nothing. Pre-fix that delta is 323px (324 on mobile); post-fix it is 0, on any baseline. `pageOverflow` splits out of `assertNoHorizontalOverflow` so a spec can compare two renders instead of asserting a global property.
Owner
Author
|
Updated the regression test after the first CI run. The original spec asserted absolute zero horizontal overflow on the detail page. CI failed it with received: 35 — the exact figure That is useful information rather than noise: the 35px is a baseline property of the book detail page in the CI environment, not something the tags cause. It is not the tag row (this render's tags were synthetic), and it is not So the assertion was wrong for what this PR owns. It now measures the delta the long tags add — render with the book's own tags, measure; swap in a long LoC heading plus an unbroken token, measure again; require the difference to be ≤1px. That holds on any baseline and still fails hard pre-fix:
The separate 35px is written up in |
new-usemame
added a commit
that referenced
this pull request
Jul 27, 2026
new-usemame
added a commit
that referenced
this pull request
Jul 27, 2026
This was referenced Jul 27, 2026
new-usemame
added a commit
that referenced
this pull request
Jul 27, 2026
#1176) Supersedes #1168, which went CONFLICTING because 41905a9 (the #1171 tag-pill fix) had already carried the v4.1.22 CHANGELOG header and whatsNew block onto main. Moves the #1170 entry out of [Unreleased] into the v4.1.22 Fixed section, adds the matching What's New item so the shipped SPA documents the build it ships in, and backfills the release cell on the #1171 CHANGES row. [Unreleased] is now empty, which is the correct pre-tag state. Docs and release-data only, no production code path. tsc --noEmit clean, SPA build clean, test_whats_new_spa.py + test_533_version_release_link.py 16/16 green, Test Suite Summary green. The advisory E2E red is the pre-existing ~35px CI-baseline detail-page overflow tracked in notes/e2e-mobile-overflow-ci-triage.md, re-measured independently this tick at 0px locally on both the guest and admin paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ships fix for #1170.
Symptom
On a phone, opening a book whose tags include a long subject heading makes the whole page scroll left/right. Hits readers browsing without an editor account — guests (anonymous browsing) and viewer/download-only accounts.
Any library carrying Library-of-Congress headings is affected, which is most metadata imported from public-domain sources. Real tags from a stock library:
France -- History -- Revolution, 1789-1799 -- Fiction,London (England) -- History -- 18th century -- Fiction.Root cause
frontend/src/components/Pill.module.csssetwhite-space: nowrapwith nomax-width. The pill sits in.tags, aflex-wrap: wraprow 366px wide at a 390px viewport — but a nowrap pill's intrinsic width is the full unbroken string, so it rendered 390px and pushed the document past the viewport.Pinned rather than assumed: with
nowrapretained, addingmax-width: 100%alone leaves the overflow unchanged at 82px. Droppingnowraptakes it to 0. The cause is the unbreakable line, not a missing cap.Why the existing gate missed it
BookDetailrenders two tag rows.canEdit(i.e.me.role.edit) gets removable.tagChipchips, which wrap fine. Everyone else gets the read-onlyPill. The e2e harness logs in asadmin, so every mobile-overflow spec exercised only the chip branch. Verified directly: as admin the page measures 0 overflow even at 320px; as guest the same page overflows.Fix
Drop
nowrap; addmax-width: 100%andoverflow-wrap: anywhere. Normal wrapping only breaks a line that doesn't fit, so pills that fit are visually unchanged (measured: short pills stay 29px tall, single line); only over-long ones wrap to two.anywherehandles a single token wider than the row.Pillhas exactly one consumer (BookDetailtags), so blast radius is that row.Verification
Live A/B on
cwn-local, guest session, book with LoC headings:Red/green on the new spec, against the real container:
expect(overflow).toBeLessThanOrEqual(1)→ received 323, failed.docker cpintocwn-local, restart to healthy: passed on bothdesktopandmobile.Also run: full
book-detail.spec.tson both projects — green.hidden-books.spec.ts:252(the mobile-overflow spec) in isolation — green.Screenshot after the fix (guest, 390px) shows the long tag wrapping to two lines inside the pill, short pills unchanged, no horizontal scroll.
The regression test stubs both
auth/me(drops the edit role) and the book payload (injects a long LoC heading + an unbroken token), so it doesn't depend on the fixture carrying long tags.Not covered by this PR
The E2E job on #1168 reports three failures, one being
hidden-books.spec.ts:252at 35px. This PR is not demonstrated to fix that. That spec runs as admin, and the admin branch measures 0 locally; I could not reproduce the 35px against the local library, and CI's 3-book fixture differs. Written up innotes/e2e-mobile-overflow-ci-triage.mdfor a follow-up tick — flagging it rather than letting the fix imply a green gate it didn't earn.Tier
Fork-original, 7 lines of production CSS in one file, plus a test. No new dependencies, licenses, or external URLs. No auth/session/CSRF, crypto, subprocess, file-path, or new-route surface, so
/security-reviewdoes not apply.